Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add implementation for Service Journey filter #5641

Closed
wants to merge 1 commit into from
Closed

Add implementation for Service Journey filter #5641

wants to merge 1 commit into from

Conversation

Bartosz-Kruba
Copy link
Contributor

@Bartosz-Kruba Bartosz-Kruba commented Jan 24, 2024 •
edited
Loading

Summary

This PR adds implemnetation for the Service Journey input in the new filter API. Previously we did not had this functionality and the API was broken. No results were returned if user tried to perform a search with Service Journey as filter input.

Issue

N/A

Unit tests

Included new unit tests for the functionality

Documentation

N/A

@Bartosz-Kruba Bartosz-Kruba force-pushed the 67952-service-journey-filter branch from faf6205 to a42cf40 Compare January 24, 2024 11:23
@Bartosz-Kruba Bartosz-Kruba marked this pull request as ready for review January 25, 2024 15:08
@Bartosz-Kruba Bartosz-Kruba requested a review from a team as a code owner January 25, 2024 15:08
@Bartosz-Kruba Bartosz-Kruba force-pushed the 67952-service-journey-filter branch from a42cf40 to 33a74fa Compare January 26, 2024 10:56
@codecov Codecov
Copy link

codecov bot commented Jan 26, 2024 •
edited
Loading

Codecov Report

Attention: 136 lines in your changes are missing coverage. Please review.

Comparison is base (e2ba239) 67.50% compared to head (33a74fa) 67.52%.
Report is 307 commits behind head on dev-2.x.

Files Patch % Lines
.../vectortiles/GraphInspectorVectorTileResource.java 0.00% 15 Missing ⚠️
...lanner/inspector/vector/edge/EdgeLayerBuilder.java 0.00% 12 Missing ⚠️
...m/filterchain/ItineraryListFilterChainBuilder.java 76.00% 9 Missing and 3 partials ⚠️
...er/inspector/vector/vertex/VertexLayerBuilder.java 0.00% 11 Missing ⚠️
...nner/inspector/vector/edge/EdgePropertyMapper.java 0.00% 9 Missing ⚠️
.../inspector/vector/vertex/VertexPropertyMapper.java 0.00% 9 Missing ⚠️
...ext/restapi/mapping/LegacyBicycleOptimizeType.java 50.00% 6 Missing ⚠️
...rithm/mapping/RouteRequestToFilterChainMapper.java 28.57% 5 Missing ⚠️
...rg/opentripplanner/visualizer/GraphVisualizer.java 0.00% 5 Missing ⚠️
...ripplanner/ext/restapi/mapping/TripTimeMapper.java 0.00% 4 Missing ⚠️
... and 24 more
Additional details and impacted files 
@@              Coverage Diff              @@
##             dev-2.x    #5641      +/-   ##
=============================================
+ Coverage      67.50%   67.52%   +0.02%     
- Complexity     16286    16295       +9     
=============================================
  Files           1887     1887              
  Lines          71555    71616      +61     
  Branches        7405     7423      +18     
=============================================
+ Hits           48303    48362      +59     
+ Misses         20750    20744       -6     
- Partials        2502     2510       +8

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@t2gran t2gran added this to the 2.5 (next release) milestone Jan 30, 2024
leonardehrenfried
leonardehrenfried reviewed Feb 5, 2024
View reviewed changes
Copy link
Member

@leonardehrenfried leonardehrenfried left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code looks good but I'm unsure of the performance impact of it.

Can you run a speed test with it? If you don't know how to do it, please get in touch.

@Bartosz-Kruba Bartosz-Kruba marked this pull request as draft February 15, 2024 15:01
@Bartosz-Kruba Bartosz-Kruba added the Stale This issue is stale, no activity for 90 days. Remove stale label or comment within 30 days. label Feb 15, 2024
@t2gran t2gran removed the Stale This issue is stale, no activity for 90 days. Remove stale label or comment within 30 days. label Mar 12, 2024
@t2gran t2gran modified the milestones: 2.5 (next release), Parked Mar 12, 2024
@t2gran
Copy link
Member

t2gran commented Mar 12, 2024

@Bartosz-Kruba I removed the Stale tag - used by the robot to tag old issues. I have instead set the milestone to Parked which we tag PRs we what to keep, but do not know when we have time to come back to. I hope this is ok.

@habrahamsson-skanetrafiken
Copy link
Contributor

Closing this after conferring with @Bartosz-Kruba
We'll solve this after #5713 gets merged instead since that PR is reworking this piece of code anyways.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@leonardehrenfried leonardehrenfried leonardehrenfried left review comments

@t2gran t2gran Awaiting requested review from t2gran

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
Parked
Development

Successfully merging this pull request may close these issues.
4 participants
@Bartosz-Kruba @t2gran @habrahamsson-skanetrafiken @leonardehrenfried